Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed wrong rotation for flipped images in auto_rotate filter #639

Closed
wants to merge 10 commits into from

Conversation

Heshyo
Copy link

@Heshyo Heshyo commented Oct 12, 2015

The original code was not rotating (nor flipping) flipped images.

The code has been tested using those images https://github.com/recurser/exif-orientation-examples

@lsmith77 lsmith77 added the State: Reviewing This item is being reviewed to determine if it should be accepted. label Oct 17, 2015
@lsmith77
Copy link
Contributor

can you add a test case?

@Heshyo
Copy link
Author

Heshyo commented Oct 28, 2015

I was able to setup my test environment but now I'm having issues faking the metadata object.

$metaData = $this->getMock('Metadata\MetadataInterface');

$metaData
    ->expects($this->once())
    ->method('offsetGet')
    ->willReturn("1")
;

$metaData->offsetGet();
// Call to undefined method Mock_MetadataInterface_7f3f507c::offsetGet()

Am I missing something obvious? I don't even know where MetadataInterface is defined (I see in ImageInterface that metadata is supposed to return a Metadata\MetadataInterface).

@lsmith77
Copy link
Contributor

this interface is part of the "jms/metadata" composer package I guess .. but what makes you believe you need to mock this interface for a test?

@Heshyo
Copy link
Author

Heshyo commented Oct 29, 2015

The autorotate needs to get the metadata from the image to know the rotation. So I want to mock it, to avoid having to provide real pictures.

I found the info on https://imagine.readthedocs.org/en/latest/usage/metadata.html. The metadata function returns a Imagine\Image\Metadata\MetadataBag.

@Heshyo
Copy link
Author

Heshyo commented Oct 29, 2015

I added the test cases and followed StyleCI recommendations.

@lsmith77
Copy link
Contributor

the style issues were already fixed in master .. it would be best if you start a new branch from current master and cherry pick your changes into there.

…/or flip the image

- Task: refactored the tests to more easily see which tests fail
@Heshyo
Copy link
Author

Heshyo commented Oct 29, 2015

Actually I only fixed the StyleCI issues for the files I modified.
I still created a new PR #654

@Heshyo Heshyo closed this Oct 29, 2015
@lsmith77 lsmith77 removed the State: Reviewing This item is being reviewed to determine if it should be accepted. label Oct 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants